Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Nuke Celery #4521

Merged
merged 38 commits into from
Jan 12, 2020
Merged

Nuke Celery #4521

merged 38 commits into from
Jan 12, 2020

Conversation

rauchy
Copy link
Contributor

@rauchy rauchy commented Jan 5, 2020

What type of PR is this? (check all applicable)

  • Refactor

Description

Following #4413, we're 100% on RQ. This PR is strictly cleanup.
@jezdez - I'd love to get some context / suggestions from you regarding how to proceed with the periodic tasks. I didn't mean to be rude when I deleted them in this PR, I just wanted to remove as much as possible.

Related Tickets & Documents

#4413 #4092

Omer Lachish added 30 commits November 13, 2019 22:59
@rauchy rauchy requested review from jezdez and arikfr January 5, 2020 10:16
@restyled-io restyled-io bot mentioned this pull request Jan 5, 2020
@jezdez
Copy link
Member

jezdez commented Jan 5, 2020

@rauchy For sure I’ll take a look tomorrow when I’m back at the computer. I’m happy to provide code to replace the periodic tasks with RQ ones, this is a pretty important API for us. Thanks for reaching out!

@rauchy
Copy link
Contributor Author

rauchy commented Jan 6, 2020

@jezdez great, thank you! If you don't mind, I think it might be better to submit it as a standalone PR.


return json_response(response)


@routes.route("/api/admin/queries/rq_status", methods=["GET"])
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Maybe we should switch the URLs not to leak the implementation detail (RQ) now that it's only RQ?

@@ -18,9 +14,6 @@
redis_connection,
rq_redis_connection,
)
from redash.metrics import celery as celery_metrics # noqa
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need to remove redash/metrics/celery.py as well.

@arikfr
Copy link
Member

arikfr commented Jan 10, 2020

@rauchy in your opinion, anything blocking us from merging this?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants